Skip to content

Conversation

@harshil-sanghvi
Copy link
Contributor

Fixed validation bug where generator expression was not being evaluated.

Changed from checking generator object to using all() to properly validate
all messages are instances of HumanMessage, AIMessage, or ToolMessage.

Added tests to verify validation works correctly.

Problem Description

Problem: The MultiTurnSample.validate_user_input() method had a critical validation bug where the generator expression was not being properly evaluated. The code was checking if not (isinstance(m, ...) for m in messages): which creates a generator object that is always truthy, causing the validation to never trigger.

Impact: This meant that invalid message types could potentially pass validation if they somehow bypassed Pydantic's type checking, though in practice Pydantic's Union validation catches most cases before this validator runs. However, the validator logic itself was fundamentally broken and would not work correctly if called.

How to replicate: The bug can be seen in the code at src/ragas/dataset_schema.py:131-133 where the generator expression without all() would never properly validate the messages.

Changes Made

  • Fixed validation logic in src/ragas/dataset_schema.py: Changed if not (isinstance(m, ...) for m in messages): to if not all(isinstance(m, ...) for m in messages): to properly evaluate all message type checks
  • Added comprehensive tests in tests/unit/test_dataset_schema.py:
    • test_multiturn_sample_validate_user_input_invalid_type(): Verifies that invalid message types are properly rejected
    • test_multiturn_sample_validate_user_input_valid_types(): Verifies that valid message types are properly accepted

References

  • File changed: src/ragas/dataset_schema.py (line 131-133)
  • Tests added: tests/unit/test_dataset_schema.py (lines 201-226)
  • Related code: The validator is part of the MultiTurnSample class which is used throughout the codebase for multi-turn conversation evaluation

   Fixed validation bug where generator expression was not being evaluated.
   Changed from checking generator object to using all() to properly validate
   all messages are instances of HumanMessage, AIMessage, or ToolMessage.

   Added tests to verify validation works correctly.
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Nov 15, 2025
Copy link
Member

@anistark anistark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @harshil-sanghvi
Looks good overall.

Can you check why CI is failing.

run make run-ci locally and you can fix it.

@harshil-sanghvi
Copy link
Contributor Author

@anistark make run-ci passes locally
======================== 579 passed, 13 skipped, 2 warnings in 27.94s =========================
All CI checks passed!

@anistark
Copy link
Member

@anistark make run-ci passes locally ======================== 579 passed, 13 skipped, 2 warnings in 27.94s ========================= All CI checks passed!

The ci is still failing. So, it's missing something. Check the failed job. You might get some ideas.

@anistark
Copy link
Member

@harshil-sanghvi Might be unrelated issue, please rebase with main. The ci should have been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants